Skip to content

Add utility from_raw_bgr{,a} for ImageBuffer#2596

Merged
197g merged 1 commit intomainfrom
from-bgr
Dec 11, 2025
Merged

Add utility from_raw_bgr{,a} for ImageBuffer#2596
197g merged 1 commit intomainfrom
from-bgr

Conversation

@197g
Copy link
Copy Markdown
Member

@197g 197g commented Sep 4, 2025

It is one of the most common channel orders other than Rgb and the construction involves nothing complicated, just swapping the channels around. We don't support that layout during computation, where it would complicate significant parts of Pixel, but as a helper for transferring a buffer we can easily have both on the Rgb{,A} typed containers.

See: #2595 which seems like a paper cut that can be easily and confidently solved.

It is one of the most common channel orders other than Rgb and the
construction involves nothing complicated, just swapping the channels
around. We don't support that layout during computation, where it would
complicate significant parts of `Pixel`, but as a helper for transferring
a buffer we can easily have both on the `Rgb{,A}` typed containers.
@197g 197g merged commit fb7f987 into main Dec 11, 2025
32 checks passed
@197g 197g deleted the from-bgr branch December 11, 2025 17:41
@fintelia
Copy link
Copy Markdown
Contributor

Do you know whether the trait bounds would be suitable if we wanted to replace the implementation with one using std::simd once that's stable? IIRC swizzles like this are something that LLVM tends not to autovectorize well, and it would be nice to make sure we're not ruling out a faster implementation later.

@197g
Copy link
Copy Markdown
Member Author

197g commented Dec 11, 2025

Hm, alright, likely not. std::simd::Simd checks the trait bound validities on the type itself not only for operations. So we could not use them here and the best alternative could only do as_chunks_mut. I suppose using the sealed PixelWithColorType would work and support most relevant buffer types.

@fintelia
Copy link
Copy Markdown
Contributor

Actually, if we made Subpixel a sealed trait (which I've been meaning to propose anyway) would that be sufficient?

@197g
Copy link
Copy Markdown
Member Author

197g commented Dec 11, 2025

From what I understand, yes. The bound would be extremely weird but it seems feasible under the current interface. It does not seem very controversial to even expose Subpixel: SimdElement publicly but if we wanted to avoid it, we'd need hacky code.

I'm not entirely sold on sealing the Subpixel trait as a mechanism of ensuring we only need to handle a finite number of them; but sure, open for a sketch that can demonstrate any of the imageops being nicer to implement as-if in a minor version.

197g added a commit that referenced this pull request Dec 11, 2025
These new bounds ensure that only our own pixel types can be used. That
prepares changing our approach of algorithms such that we can optimize
for individual cases without falling prey to needs for specialization.

More context can be found here:

<#2596 (comment)>
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Release 0.25.* Mar 1, 2026
197g added a commit that referenced this pull request Mar 8, 2026
These new bounds ensure that only our own pixel types can be used. That
prepares changing our approach of algorithms such that we can optimize
for individual cases without falling prey to needs for specialization.

More context can be found here:

<#2596 (comment)>
@197g 197g mentioned this pull request Mar 9, 2026
@197g 197g moved this from Done to In progress in Release 0.25.* Mar 9, 2026
@197g 197g moved this from In progress to Backported in Release 0.25.* Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backported

Development

Successfully merging this pull request may close these issues.

2 participants